Skip to content

CM-1112: Fix cluster-monitoring set as annotation instead of label#437

Open
sebrandon1 wants to merge 1 commit into
openshift:masterfrom
sebrandon1:fix-cluster-monitoring-label
Open

CM-1112: Fix cluster-monitoring set as annotation instead of label#437
sebrandon1 wants to merge 1 commit into
openshift:masterfrom
sebrandon1:fix-cluster-monitoring-label

Conversation

@sebrandon1

@sebrandon1 sebrandon1 commented Jun 10, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Summary

  • Moves openshift.io/cluster-monitoring: "true" from annotations to labels on the cert-manager namespace manifest
  • Regenerates bindata.go to match

Per the OCP docs, cluster monitoring requires the openshift.io/cluster-monitoring: "true" key to be set as a label on the namespace. The operator was incorrectly setting it as an annotation, which prevented Prometheus from discovering the namespace for metrics scraping.

Note on existing clusters: The operator's static resource controller (library-go EnsureObjectMeta) merges labels additively. On upgrade, the label will be added but the old annotation will remain. This is harmless — monitoring only checks for the label.

Cluster verification (OCP 4.19)

Before — label missing, only annotation set:

$ oc get ns cert-manager -o jsonpath='{.metadata.annotations.openshift\.io/cluster-monitoring}'
true

$ oc get ns cert-manager -o jsonpath='{.metadata.labels.openshift\.io/cluster-monitoring}'
(empty)

After — label correctly set:

$ oc get ns cert-manager -o jsonpath='{.metadata.labels.openshift\.io/cluster-monitoring}'
true

$ oc get ns cert-manager -o jsonpath='{.metadata.labels}'
{
    "kubernetes.io/metadata.name": "cert-manager",
    "openshift.io/cluster-monitoring": "true",
    "pod-security.kubernetes.io/audit": "restricted",
    "pod-security.kubernetes.io/audit-version": "latest",
    "pod-security.kubernetes.io/warn": "restricted",
    "pod-security.kubernetes.io/warn-version": "latest"
}

The old annotation remains on existing clusters (additive merge), which is harmless:

$ oc get ns cert-manager -o jsonpath='{.metadata.annotations.openshift\.io/cluster-monitoring}'
true

Fixes #336

Test plan

  • Unit tests pass (make test)
  • Verified on live OCP 4.19 cluster: label present after applying fix
  • CI e2e tests pass

Summary by CodeRabbit

  • Chores
    • Updated the cert-manager namespace manifest so the cluster monitoring setting is now applied via metadata.labels instead of metadata.annotations.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 10, 2026
@openshift-ci-robot

openshift-ci-robot commented Jun 10, 2026

Copy link
Copy Markdown

@sebrandon1: This pull request explicitly references no jira issue.

Details

In response to this:

Summary

  • Moves openshift.io/cluster-monitoring: "true" from annotations to labels on the cert-manager namespace manifest
  • Regenerates bindata.go to match

Per the OCP docs, cluster monitoring requires the openshift.io/cluster-monitoring: "true" key to be set as a label on the namespace. The operator was incorrectly setting it as an annotation, which prevented Prometheus from scraping cert-manager metrics.

Note on existing clusters: The operator static resource controller (library-go EnsureObjectMeta) merges labels additively. On upgrade, the label will be added but the old annotation will remain. This is harmless -- monitoring only checks for the label.

Fixes #336

Test plan

  • Unit tests pass (make test)
  • Verify oc get ns cert-manager -o jsonpath='{.metadata.labels}' includes openshift.io/cluster-monitoring: "true" after operator reconciles
  • Verify cert-manager metrics appear in cluster monitoring after the fix

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: bad05e59-59ce-4c51-a44a-ced5030056fb

📥 Commits

Reviewing files that changed from the base of the PR and between e6de6a6 and c60c5de.

📒 Files selected for processing (2)
  • bindata/cert-manager-deployment/cert-manager-namespace.yaml
  • pkg/operator/assets/bindata.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • bindata/cert-manager-deployment/cert-manager-namespace.yaml
  • pkg/operator/assets/bindata.go

Walkthrough

The cert-manager Namespace manifest and its embedded bindata asset now set openshift.io/cluster-monitoring: "true" under metadata.labels instead of metadata.annotations.

Changes

Cluster monitoring label placement

Layer / File(s) Summary
Namespace label relocation
bindata/cert-manager-deployment/cert-manager-namespace.yaml, pkg/operator/assets/bindata.go		 openshift.io/cluster-monitoring: "true" is moved from metadata.annotations to metadata.labels in the Namespace manifest and matching embedded asset bytes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: moving cluster-monitoring from an annotation to a label.
Linked Issues check ✅ Passed The PR implements #336 by placing openshift.io/cluster-monitoring under metadata.labels and updating generated assets.
Out of Scope Changes check ✅ Passed The changes stay focused on the namespace manifest and regenerated bindata, with no unrelated edits.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Stable And Deterministic Test Names ✅ Passed PR only updates YAML and generated bindata; no Ginkgo tests were added or modified, so there are no unstable test titles to flag.
Test Structure And Quality ✅ Passed PASS: The PR only changes a namespace manifest and regenerated bindata; no Ginkgo test code was added or modified, so the checklist doesn't apply.
Microshift Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the patch only updates a Namespace YAML manifest and regenerated bindata, so MicroShift compatibility is unaffected.
Single Node Openshift (Sno) Test Compatibility ✅ Passed Only the namespace YAML and generated bindata changed; no new Ginkgo e2e tests or multi-node assumptions were added.
Topology-Aware Scheduling Compatibility ✅ Passed Only the cert-manager namespace label moved in manifest and regenerated bindata; no pod scheduling, affinity, replicas, or node selectors changed.
Ote Binary Stdout Contract ✅ Passed The PR only changes namespace YAML and generated bindata; no stdout-writing calls were found in process-level entrypoints.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the PR only changes a namespace manifest and regenerated bindata, so no IPv4/external connectivity assumptions apply.
No-Weak-Crypto ✅ Passed Diff only moves a namespace label and regenerates bindata; no weak crypto, custom crypto, or secret/token comparisons were added.
Container-Privileges ✅ Passed Only a Namespace label changed; the touched manifest and generated asset contain no privileged/hostPID/hostNetwork/hostIPC/SYS_ADMIN or allowPrivilegeEscalation=true settings.
No-Sensitive-Data-In-Logs ✅ Passed Touched files only change the namespace label in YAML/bindata; no log calls or sensitive-data patterns were found.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@openshift-ci openshift-ci Bot requested review from bharath-b-rh and swghosh June 10, 2026 16:37
@openshift-ci

openshift-ci Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sebrandon1
Once this PR has been reviewed and has the lgtm label, please assign bharath-b-rh for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sebrandon1 sebrandon1 changed the title NO-JIRA: Fix cluster-monitoring set as annotation instead of label CM-1112: Fix cluster-monitoring set as annotation instead of label Jun 10, 2026
@openshift-ci-robot

openshift-ci-robot commented Jun 10, 2026 •
edited by openshift-ci Bot
Loading

Copy link
Copy Markdown

@sebrandon1: This pull request references CM-1112 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

  • Moves openshift.io/cluster-monitoring: "true" from annotations to labels on the cert-manager namespace manifest
  • Regenerates bindata.go to match

Per the OCP docs, cluster monitoring requires the openshift.io/cluster-monitoring: "true" key to be set as a label on the namespace. The operator was incorrectly setting it as an annotation, which prevented Prometheus from discovering the namespace for metrics scraping.

Note on existing clusters: The operator's static resource controller (library-go EnsureObjectMeta) merges labels additively. On upgrade, the label will be added but the old annotation will remain. This is harmless — monitoring only checks for the label.

Cluster verification (OCP 4.19)

Before — label missing, only annotation set:

$ oc get ns cert-manager -o jsonpath='{.metadata.annotations.openshift\.io/cluster-monitoring}'
true

$ oc get ns cert-manager -o jsonpath='{.metadata.labels.openshift\.io/cluster-monitoring}'
(empty)

After — label correctly set:

$ oc get ns cert-manager -o jsonpath='{.metadata.labels.openshift\.io/cluster-monitoring}'
true

$ oc get ns cert-manager -o jsonpath='{.metadata.labels}'
{
   "kubernetes.io/metadata.name": "cert-manager",
   "openshift.io/cluster-monitoring": "true",
   "pod-security.kubernetes.io/audit": "restricted",
   "pod-security.kubernetes.io/audit-version": "latest",
   "pod-security.kubernetes.io/warn": "restricted",
   "pod-security.kubernetes.io/warn-version": "latest"
}

The old annotation remains on existing clusters (additive merge), which is harmless:

$ oc get ns cert-manager -o jsonpath='{.metadata.annotations.openshift\.io/cluster-monitoring}'
true

Fixes #336

Test plan

  • Unit tests pass (make test)
  • Verified on live OCP 4.19 cluster: label present after applying fix
  • CI e2e tests pass

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@sebrandon1 sebrandon1 force-pushed the fix-cluster-monitoring-label branch 2 times, most recently from f7307e6 to 7987812 Compare June 15, 2026 16:25
@sebrandon1 sebrandon1 force-pushed the fix-cluster-monitoring-label branch from 7987812 to e6de6a6 Compare June 18, 2026 17:41
@sebrandon1
CM-1112: Fix cluster-monitoring set as annotation instead of label
c60c5de 
The openshift.io/cluster-monitoring key was incorrectly set as an
annotation on the cert-manager namespace instead of a label. OpenShift
cluster monitoring requires this to be a label in order to scrape
metrics from the namespace.

Fixes openshift#336
@sebrandon1 sebrandon1 force-pushed the fix-cluster-monitoring-label branch from e6de6a6 to c60c5de Compare June 24, 2026 19:51
@openshift-ci

openshift-ci Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

@sebrandon1: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-operator-tech-preview c60c5de link false /test e2e-operator-tech-preview
ci/prow/e2e-operator c60c5de link true /test e2e-operator

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bharath-b-rh bharath-b-rh Awaiting requested review from bharath-b-rh
@swghosh swghosh Awaiting requested review from swghosh

Assignees

No one assigned

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

openshift.io/cluster-monitoring incorrectly set as annotation instead of a label

2 participants

@sebrandon1 @openshift-ci-robot